Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document AStop #2816

Merged
merged 6 commits into from
Nov 1, 2024
Merged

Document AStop #2816

merged 6 commits into from
Nov 1, 2024

Conversation

Kevin-OConnor
Copy link
Contributor

Add documentation for new DS AStop to DS article and beta testing task

Add documentation for new DS AStop to DS article and beta testing task
Allows GIFs to be used online and converted to PNG for PDF
@Kevin-OConnor Kevin-OConnor marked this pull request as draft October 28, 2024 21:37
@Kevin-OConnor
Copy link
Contributor Author

Added the imgconverter extension to make sure it works properly in CI, but if we go this direction I should also add the ImageMagick dependency to the Build Instructions page before merging. With it being an easily available open source tool and only needed for PDF I don't think it's a big deal to add. Thoughts @Daltz333 or @sciencewhiz ?

@sciencewhiz
Copy link
Collaborator

There's a similar discussion happening in #2774

@TheTripleV
Copy link
Member

I think no one builds pdf locally so a new dep for pdf is fine.

This affects Sam's pr with gifs too.
Is the repo standard to use mp4 instead of gifs everywhere for filesize?

@sciencewhiz
Copy link
Collaborator

Is the repo standard to use mp4 instead of gifs everywhere for filesize?

Both for filesize and for accessibility. It's not possible to disable an animated gif. If someone were to be sensitive to the blinking in this screenshot, they wouldn't be able to disable it, unlike a video.

@sciencewhiz
Copy link
Collaborator

sciencewhiz commented Oct 29, 2024

whoops, meant to push to my fork for testing, guess we'll see if mp4s with only directives works here. mp4 is 1/4 the size of the gif

@sciencewhiz
Copy link
Collaborator

Everything looks to be working with mp4

@Kevin-OConnor Kevin-OConnor marked this pull request as ready for review October 31, 2024 13:54
@jasondaming
Copy link
Member

I think the visualization is a great idea and we should do more of that.

For this one however I had trouble following it mostly because I didn't understand when it stopped and where it began. Because it auto replayed and there was little difference between the start and end states it looped seamlessly for me. Two solutions I can think of: is there a way we can make is wait 5? seconds before repeating? or make is a little longer at the end so that it is more obvious when it repeats.

Update video to make behavior more obvious and update Practice description to include this.
Changes PDF image to figure to add a caption to go to web for animation.
@Kevin-OConnor
Copy link
Contributor Author

Kevin-OConnor commented Oct 31, 2024

For this one however I had trouble following it mostly because I didn't understand when it stopped and where it began.

Re-recorded longer and with showing the practice timing at the beginning so the loop is more obvious. Does that help @jasondaming ?

@jasondaming
Copy link
Member

Was it shorter than 7 seconds before? That is the video I see currently.

Not a big deal the orange blinking is the primary thing you are trying to highlight.

@Kevin-OConnor
Copy link
Contributor Author

Was it shorter than 7 seconds before? That is the video I see currently.

Not a big deal the orange blinking is the primary thing you are trying to highlight.

That might be a cache issue? I see the updated 14s video showing here: https://frc-docs--2816.org.readthedocs.build/en/2816/docs/software/driverstation/driver-station.html

@Kevin-OConnor Kevin-OConnor merged commit a6c26b1 into wpilibsuite:main Nov 1, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants